Skip to content

test: an involuntary skip is a failure — tools/_skip_policy.py and the rig lane (#178a) - #180

Merged
JC-000 merged 5 commits into
masterfrom
test/178a-skip-policy-rig-half
Sep 6, 2026
Merged

test: an involuntary skip is a failure — tools/_skip_policy.py and the rig lane (#178a)#180
JC-000 merged 5 commits into
masterfrom
test/178a-skip-policy-rig-half

Conversation

@JC-000

@JC-000 JC-000 commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Part A of #178. Codifies the project's involuntary-skip rule as a callable and wires the rig lane to it.

Fourth revision. Three adversarial review rounds. Round 1 found five defects (all reproduced, all fixed in 8cc2d1c); round 2 verified those five and found one more — a whole-PR claim in this body that was false by omission — fixed in 76621b6; round 3 verified everything and found one stale comment, fixed in 4e90ffc. Corrected claims are rewritten below rather than quietly patched.

What it does

Adds tools/_skip_policy.py — three deliberately different calls, because the remedy decides the verdict:

call verdict exit remedy
not_applicable() voluntary — another rig owns the coverage 0, but as a named verdict, never a bare return 0 "use the other rig"
cannot_run() involuntary — this IS the target config and it still could not run 2, distinct from 1 ("a check ran and failed") "install something"
cannot_run(opt_out_env=None) the things no env var may silence: a failed build, and contention 2, no hatch

Wired into eight entry points: the four tests/rig_phase*.py bridge rigs, tests/rig_vice_https_macos.py, tools/test_p384_symbols.py, tools/test_ecdsa_p384_kat.py, and tools/https_e2e/ (which gains a platform_supported() predicate so check_prerequisites()'s contract is unchanged).

Two new pure-logic pytest modules, both in pytest.ini testpaths: tools/test_skip_policy.py (19 cases, the helper) and tools/test_rig_skip_contract.py (12 cases, the call sites — a helper with no call-site test is just another convention to miss, which is #178's own thesis).

Why

Measured before the change, with build/ absent:

  • tests/rig_phase{1_dhcp,2_http,3_https,3_https_1mhz}.py — exit 0 at the missing-prerequisite site and at return _skip("c64-https.prg could not be built"). A failed make laundered into a pass.
  • tests/rig_vice_https_macos.py — exit 0 with the rig down; and SystemExit("build failed") exited 1, making a broken build indistinguishable from a real handshake failure.
  • tools/test_p384_symbols.py — exit 0 on BACKEND != uci (voluntary, but silent).
  • tools/test_ecdsa_p384_kat.py --u64 with no U64_HOST — printed U64: 0/0 passed and OVERALL: PASS, exit 0.

Behaviour changes, both directions, stated rather than discovered:

  • Tightening. --u64 without U64_HOST is now refused up front, before the multi-hour VICE lane, and fails in ~0.1 s. It is not opt-out-able — see F1 in round 2 for why that matters.
  • No loosening. Two gates in the first push did get weaker than master, in opposite corners of the tree; both are reverted (defect D3 in round 1, F1 in round 2). With those closed, no exit code in this PR is weaker than master's — and that claim has now been swept exhaustively by the reviewer across all seven wired files, master against branch, every return / SystemExit / sys.exit including every opt-out path.

Platform is asked FIRST and is a voluntary skip: the four bridge rigs are Linux-only, and on Darwin check_prerequisites() reports "ip not on PATH; iptables not on PATH", which is not installable there. Routing that to cannot_run() would make four rigs a permanent red on the project's primary platform, with the global opt-out as the only remedy — i.e. the policy gets disabled everywhere.

The opt-out requires the literal "1". Bare truthiness meant C64_ALLOW_SKIP=0 enabled it, so setting 0 to close the hatch disabled the whole policy. Every other gate in this repo (C64_SKIP_BUILD, VICE_HTTPS_OK_TO_RUN, …) compares to "1" and this one now does too.

Also narrowed the macOS rig's contention detector: pgrep -fl "ethernetioif feth0" matched the full command line of every process against an unanchored pattern (a grep, an editor, a driver script carrying that text all matched — demonstrated with a plain python3 -c). Now pgrep -x x64sc plus an argv check.

Review round 1 — five defects, all reproduced, all fixed in 8cc2d1c

D1. require()'s opt-out crashed outside pytest — and the hand-rolled copy it supersedes had already fixed this. import pytest succeeds whenever pytest is installed, never mind whether it is driving, and pytest.outcomes.Skipped derives from BaseException, which neither runner's except Exception can catch. With the sibling harness blocked:

$ C64_ALLOW_SKIP=1 python3 tools/test_rig_skip_contract.py
  ...
  File "tools/_skip_policy.py", line 279, in require
    pytest.skip(f"{text} [{opt_out_env}=1 set]")
Skipped: COULD NOT RUN: tools/https_e2e could not be imported ...
EXIT=1

No summary line, and the four source-inspection tests that could have run never ran. Exit 1 means "a check ran and failed" under the module's own contract, so it was wrong in both directions — and only on the escape hatch this PR advertises. Without the opt-out the same run was correct (4/10 passed, six named failures), which is why nothing caught it.

tools/test_uci_data_acc.py:631 had already solved this, with sys.modules.get("pytest") and a comment saying why. A shared helper that is worse than the copy it supersedes is not a refactor. Fixed by mirroring it: outside pytest, require() raises VoluntarySkip, a plain Exception. Either way the body does not run, so fail-closed is unchanged.

D2. The literal-"1" guard was skip-swallowed by the mutation it targets. test_require_still_raises_when_the_opt_out_is_zero was except SkipPolicyError: pass — vacuous against its own defect. Under bare truthiness, bool("0") is true, require() takes the opt-out branch, pytest records SKIPPED, exit 0. Measured: 1 skipped, not a failure. The guard turned itself into a green skip under precisely the defect it guards — the vacuous-skip shape #178 exists to kill, inside #178's own implementation — and it read as caught only because three sibling tests on the cannot_run() lane happen to fail. The catch is now BaseException-wide; under the same mutant the module reports 4 failed, 13 passed.

D3. Undisclosed loosening: VICE_HTTPS_OK_TO_RUN unset went 2 → 0. Reverted to exit 2, non-opt-out-able. Unset is the default state — it cannot tell a considered decline from a forgotten flag — and what the flag asserts ("the UCI HTTPS listener has stopped") is a contention claim, the one category this policy gives no opt-out at all; the port-443 check three lines below is the same interlock measured directly and was already exit 2. The only thing #178 now changes about this gate is that the verdict is a named block instead of a bare print.

D4. tests/rig_phase2_http.py contradicted itself inside forty lines. Its docstring said the macOS rig "owns the coverage"; its own _COUNTERPART string — the one that reaches the operator — says that rig drives TLS, "so plaintext HTTP specifically has no macOS rig". Load-bearing, because the voluntary-skip verdict is justified by "nothing is lost", which for this one rig is false. Exit 0 stays (no remedy on macOS), but the reason is now the true one and a new test pins the two halves together.

D5. Both _standalone() runners collapsed "could not run" onto 1. They now separate VoluntarySkip / SkipPolicyError / Exception and return 0 / 1 / 2. Measured on tools/test_rig_skip_contract.py with the harness blocked: exit 2 plain, exit 0 with C64_ALLOW_SKIP=1 (5/11 passed, 6 skipped by explicit opt-out), exit 0 with the harness present.

Red-then-green evidence

Every mutant below was run on this branch at dc06095, macOS, pytest 9.0.3.

M0′ — revert only the four bridge rigs, keep https_e2e. The sharp version of the contract mutant:

FAILED test_bridge_rigs_ask_platform_before_prerequisites
FAILED test_bridge_rigs_still_fail_when_the_platform_is_right
FAILED test_bridge_rigs_route_a_broken_build_to_two_without_an_opt_out
FAILED test_the_interlock_flag_unset_is_contention_and_stays_exit_two
FAILED test_phase2_does_not_claim_a_counterpart_it_does_not_have
5 failed, 6 passed

These are real assertions about exit codes, e.g.

E   AssertionError: rig_phase1_dhcp: broken build must be 2 even opted out, got 0
E     SKIP: c64-https.prg could not be built
E   assert 0 == 2

M1 — the "1" comparison. Replace _opted_out's .strip() == "1" with bool(...): 4 failed, 13 passed, including test_require_still_raises_when_the_opt_out_is_zero (which skipped before D2 was fixed).

D1 mutant — restore import pytest in require()test_require_does_not_hand_a_skip_to_a_non_pytest_runner FAILS.
D3 mutant — re-route the interlock to not_applicable()test_the_interlock_flag_unset_is_contention_and_stays_exit_two FAILS.
D4 mutant — restore the contradictory docstring → test_phase2_does_not_claim_a_counterpart_it_does_not_have FAILS.

Each restored → green.

Green, whole suite. Bare pytest at the repo root, with a BACKEND=uci USE_NISTCURVES_ONCHIP=1 PRG in build/ (sha256 5f9e9fb9e70edd2a15ab2249120f161d1826262140fa6ce6f3e01410a876b474):

86 passed in 7.27s

31 of those 86 are new (origin/master is 55 passed in the same conditions).

Review round 2 — one more defect, fixed in 76621b6

Round 2 re-verified all five round-1 fixes by reproducing the original scenarios (not by reading the diff) and confirmed every number in this body. It found one real hole:

F1. "No exit code is weaker than master's" was false by omission, at tools/test_ecdsa_p384_kat.py — the one wired file D3 did not touch. Round 1 did enumerate this file's exit paths; what it missed was the opt-out interaction specifically, not the file.

--u64, U64_HOST unset
master runs the entire VICE lane, then return 0 if total_fail == 0 else 1 → a failing emulator vector reports 1
this branch, before the fix new gate fires before _build_prg() and before any VICE work, carrying opt_out_env="C64_ALLOW_SKIP" → with that set, 0, nothing run

The defect is scope, not arithmetic. C64_ALLOW_SKIP answers "this lane has no hardware" — precisely the operator who would set it here — and honouring it at a gate that precedes the VICE lane silences the emulator half too, which needs no hardware and could have run. They get exit 0 and never learn the emulator-only half regressed.

Fixed with opt_out_env=None rather than by moving the gate: the up-front refusal is the disclosed tightening (~0.1 s instead of hours), and moving it below the VICE lane gives that back. The remedy needs no environment variable — set U64_HOST, or drop --u64. "This lane has no hardware" is spelled by not passing --u64; asking for hardware and not supplying it is a malformed invocation, not a coverage gap to acknowledge. The unreachable backstop later in main() gets the same treatment so one file does not answer the same question two ways.

Severity was low — three deliberate operator choices, the block prints in full, and _build_prg() would likely fail first since the P-384 build has never completed — but the claim was checkable and did not hold, so the code moved rather than the claim.

F3. D5 was the one round-1 fix with no test. Nothing collects either _standalone(), so a future edit collapsing the split back to return 1 if failed else 0 would go unnoticed — in a PR whose thesis is that a helper with no call-site test is just another convention to miss. Two subprocess cases now drive the real runner in tools/test_rig_skip_contract.py with c64_test_harness made unimportable by a meta-path finder (the sibling is pip-installed, so there is no path entry to remove). That target cannot recurse — it has no spawner of its own. No opt-out → exit 2, CANNOT RUN + the vacuity string, summary still printed. C64_ALLOW_SKIP=1exit 0, no traceback, skipped by explicit opt-out, and the four tests that never needed the sibling still show PASS. That second case is also the D1 crash scenario, now pinned.

F2. One docstring paragraph. VoluntarySkip is an Exception, so a broad except Exception can swallow it. Not reachable today (every require() consumer names it first) and the direction is safe — a swallowed voluntary skip reports as a failure, never as a pass. The class now says it is swallowable by design and tells consumers to name it before the bare except. Deliberately not a BaseException: that is the choice that caused D1.

Round-2 red-green:

  • F1 mutant — restore opt_out_env="C64_ALLOW_SKIP" on the --u64 gate → test_the_u64_gate_is_not_opt_out_able_because_it_precedes_the_vice_lane FAILS (1 failed, 11 passed).
  • D5 mutant — collapse the runner to return 1 if failed or cannot else 0test_standalone_runner_returns_two_for_could_not_run FAILS (1 failed, 18 passed).

Review round 3 — one stale comment, fixed in 4e90ffc

Round 3 reproduced all three round-2 red-greens and confirmed every count. It found one thing, and it is a good catch: my "cannot recurse" rationale was false in the commit that wrote it.

The comment said the spawn target "has no spawner of its own". True when I reasoned it — and false by the time 76621b6 was written, because the same commit added one. test_the_u64_gate_* in test_rig_skip_contract.py spawns test_ecdsa_p384_kat.py, and _standalone() runs every test_*, so the real chain is:

pytest
  -> test_rig_skip_contract.py    (spawned by test_skip_policy.py)
       -> test_ecdsa_p384_kat.py  (spawned by its u64-gate test)

Confirmed to fire: inside the blocked standalone run the u64-gate case reports PASS, which it can only do by having spawned the KAT. No fork bomb and no hang — depth 2, terminating (the KAT spawns nothing), the whole nested run measures 0.065 s and exits 2, and the timeouts nest correctly at 180 s outer / 120 s inner.

One clause of that fix was itself wrong and is corrected in 4251c17: the comment said the chain "terminates because the P-384 KAT spawns nothing". The KAT is not a leaf in general — _build_prg() runs make clean and two make BACKEND=uci invocations at test_ecdsa_p384_kat.py:646-665. It is a leaf on this path, because the --u64 gate returns before _build_prg(). That names a coupling nobody had written down: the D5 test's cost bound rests on the F1 fix keeping that gate up front. Move it below _build_prg() — the alternative fix rejected in round 2 — and the nested run starts a full make clean && make inside the 120 s inner timeout. Two decisions one line apart, in a file that mentioned neither. Both files now say so: the chain comment names the make calls and states that moving the gate is also a decision about this test's cost, and test_rig_skip_contract.py — which adds the second link and carried no note at all — gets the chain drawn at the spawn site. A cross-file invariant documented in one of the two files is half-documented: the reader who breaks it is the one editing the other.

Nothing in the code needed to change. What needed fixing is that the comment leaned on an invariant — "no spawner exists" — which is precisely the property that would stop a third link being added later, and which was no longer true. A safety argument that has quietly gone false is worse than none, because the next reader stops looking. The comment now spells the chain out, states the depth, says the bound is by inspection with no guard in the code, gives the measured cost and the timeout nesting, and records that the earlier claim went stale inside one commit.

Merge order

#183#180#186. The ordering stands. The conflict that motivated it does not: #186 has since dropped its vendored copy of the helper entirely and rebased onto the fixed #183 tip, verified by merging three times — conflict-free, all three pytest.ini testpaths surviving, bare pytest 92 on the merged tree. Nothing is owed from this side.

Note on the build-state failures — attribution corrected

Without a build in build/, bare pytest is 10 failed, 70 passed on this branch and 10 failed, 45 passed on origin/master — the same ten, so not a regression here. The first version of this body attributed the seven test_build_flags_stamp failures to "needs ca65, that is open #177". That was wrong. ca65, ld65, ar65 and od65 were all on PATH and the seven still failed; _toolchain_missing() checks only ca65/ld65, so the #177 skip path was never taken. What fixed them was git submodule update --init libs/nistcurves libs/x25519 — a fresh worktree has no submodule working trees, so the sibling archive cannot build. #177 is a real open issue, but it is not what these failures were.

What was re-verified against current master

The commit was written at 0b55c30; #168, #172, #173, #175 and #176 have landed since. A REBASE NOTE recording this is appended to the first commit's message.

Known gap, carried from the original review

The four test_macos_rig_* cases assert on source text only, so behavioural mutants of tests/rig_vice_https_macos.py survive them. Closing that needs the rig restructured to be importable without the sibling c64_test_harness, which is not a drive-by. Stated plainly: this PR pins the four bridge rigs behaviourally and the macOS rig by shape only.

No PRG bytes change: the diff is Python and pytest.ini only.

🤖 Generated with Claude Code

Add tools/_skip_policy.py and wire the in-scope entry points to it.  Three
lanes, deliberately different calls, because the REMEDY decides the verdict:

  not_applicable() VOLUNTARY -- this host or configuration can never run
                  this rig and another rig owns the coverage, or the
                  operator declined an opt-in rig.  Nothing is lost, so
                  exit 0 -- but as a named verdict, never a bare
                  `return 0`.  Remedy: "go use the other rig", or "opt in".
  cannot_run()    INVOLUNTARY -- this IS the target platform/config and it
                  still could not run: no toolchain, no PRG, no hardware.
                  A real coverage hole, so exit 2 (distinct from 1, "a
                  check ran and failed").  Remedy: "install something".
  cannot_run(opt_out_env=None) -- the two things no environment variable
                  may silence: a failed build, and contention.

Measured before the change, with build/ absent:

  tests/rig_phase{1_dhcp,2_http,3_https,3_https_1mhz}.py   exit 0 at the
    missing-prerequisite site AND at `return _skip("c64-https.prg could
    not be built")` -- a failed `make` laundered into a pass.
  tests/rig_vice_https_macos.py                            exit 0 with the
    rig down; and `SystemExit("build failed")` exited 1, making a broken
    build indistinguishable from a real handshake failure.
  tools/test_p384_symbols.py                               exit 0 on
    BACKEND != uci (voluntary, but silent).
  tools/test_ecdsa_p384_kat.py --u64 with no U64_HOST       printed
    "U64: 0/0 passed" and OVERALL: PASS, exit 0.

BEHAVIOUR CHANGE worth stating rather than discovering: that last one is
now refused UP FRONT, before the multi-hour VICE lane, instead of at the
verdict.  `--u64` without U64_HOST fails in ~0.1 s.

Platform is asked FIRST, and it is a voluntary skip.  The four bridge rigs
are Linux-only; on Darwin check_prerequisites() reports "ip not on PATH;
iptables not on PATH", which is not installable there, so routing that to
cannot_run() would make four rigs a permanent red on the project's primary
platform -- with the global opt-out as the only remedy, i.e. the policy
gets disabled everywhere.  https_e2e gains a separate platform_supported()
predicate rather than partitioning check_prerequisites(): that function
returns a flat list of strings whose entries mean different things on
different platforms, and it is shared by four rigs and shadowed by a
same-named function in tools/test_http_integration.py.  Its contract is
unchanged.  rig_vice_https_macos.py gets the symmetric lift.

Contention is a third category.  In rig_vice_https_macos.py it is split
out of _rig_check(); in rig_phase3_https_1mhz.py the port-443 check is the
same thing.  Both exit 2 with no opt-out: a lane that silences contention
goes green exactly when it collides, and unlike a missing tool it clears
on its own.  It is evaluated AFTER `problems`, because contention on a rig
that does not exist is a meaningless headline -- and, being non-opt-out-
able, would otherwise leave a no-rig CI lane red with no recourse.

The contention detector is narrowed accordingly.  `pgrep -fl "ethernetioif
feth0"` matched the full command line of every process against an
unanchored pattern, so a grep, an editor, or a driver script carrying that
text matched too (demonstrated: a plain `python3 -c ... "ethernetioif
feth0"` matches).  It is now `pgrep -x x64sc` plus an argv check, so it
matches only a process whose executable is x64sc and whose arguments name
feth0.

rig_phase3_https_1mhz.py's two pre-flight gates predate this change but
contradicted the contract it documents: an unset VICE_HTTPS_OK_TO_RUN is
the operator declining an opt-in rig (now exit 0, named verdict), and a
held port 443 is contention (exit 2, no opt-out).  The platform gate moved
ahead of both, so a host that can never run the rig is never told to opt
in and never trips the port check.

The opt-out requires the literal "1".  Bare truthiness meant
C64_ALLOW_SKIP=0 ENABLED it, so setting 0 to close the hatch disabled the
whole policy; every other gate in this repo (C64_SKIP_BUILD,
VICE_HTTPS_OK_TO_RUN, ...) compares to "1" and this one now does too.
require() also fails closed when the opt-out is set but pytest is absent,
rather than returning into a test body whose prerequisite is missing.

Two test modules, both pinned in pytest.ini testpaths:

  tools/test_skip_policy.py       15 cases -- the "1" comparison across
    0 / false / empty / yes / true / 2 / " 1 " / "1\n", both lanes, and
    require().
  tools/test_rig_skip_contract.py 10 cases -- the CALL SITES.  A helper
    with no call-site test is just another convention to miss, which is
    #178's own thesis: nothing otherwise pinned that a rig asks
    platform_supported() BEFORE check_prerequisites(), and swapping those
    two lines silently returns macOS to exit 2 with a green suite.  Both
    ordering assertions were mutation-tested: swapping the gates in
    rig_phase1_dhcp.py trips the tripwire naming check_prerequisites, and
    swapping problems/contention in rig_vice_https_macos.py fails the
    routing assertion.

No `total_run == 0` vacuity guard is left in test_ecdsa_p384_kat.py.  One
was written and removed: _build_vector_list() is never empty and
_run_backend() counts every vector, including under --sha-only, so it
could not fire under any flag combination.  A check that matches nothing
is the shape #178 exists to close, and shipping one inside #178's own
implementation is the worst place for it.

tools/test_rig_skip_contract.py does NOT import tools/https_e2e at module
level.  That package re-exports from .vice_on_bridge, which imports
c64_test_harness from a sibling checkout, so a module-level import made bare
`pytest` on a fresh clone die with a COLLECTION ERROR -- which pytest.ini's
own comment says must never be mistaken for a passing run -- and it silently
voided the very hazard the module's docstring cites as its reason not to
import the macOS rig.  It is now loaded on first use through require(), the
API's first production call site: with c64_test_harness blocked, the module
imports cleanly and six tests FAIL by name carrying the full reason, while
the four source-inspection tests still pass.

KNOWN GAP, follow-up owed.  The four test_macos_rig_* cases assert on SOURCE
TEXT only, so behavioural mutants of tests/rig_vice_https_macos.py survive
them: reordering its gates at runtime, or changing a verdict without
changing the anchored text, would not be caught (mutants M5-M8 in review).
Closing that needs the rig restructured to be importable without the sibling
harness, which is not a drive-by.  Stated plainly: this commit pins the four
bridge rigs BEHAVIOURALLY and the macOS rig by SHAPE only.

SCOPE.  #178 stays split: Part 2 (the tools/test_pytest_boundary.py guard)
lands as #178b AFTER #172 and #177, because it codifies a rule those two
are instances of and pinning a rule while live violations remain is the
wrong order.  The consequence is explicit: this PR's guard will never be
SEEN to fail, and #178b owes the red-first demonstration.

A ninth instance of the class exists at tools/test_uci_data_acc.py:714-721
(missing PRG -> pytest.skip), in the exact lane require() was written for.
It is owned by open PR #172 and deliberately untouched here; a follow-up
is owed once that lands.  This PR's one-line pytest.ini addition was
3-way merge-tested against #172's pytest.ini: merges clean, no conflict.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

REBASE NOTE (2026-09-05, onto dc06095).  Written at 0b55c30; #168, #172,
#173, #175 and #176 have landed since.  Re-verified, and one paragraph
above is now historical rather than current:

  - #172 is MERGED, so the "ninth instance" it owned is closed.  It is
    closed by a hand-rolled implementation of this same policy inside
    tools/test_uci_data_acc.py (`_require`/`Unavailable`, the "1"
    comparison, exit 2, the reason string carrying its own vacuity
    warning), not by importing require().  The follow-up that paragraph
    says is owed is therefore now actionable: converge that module onto
    tools/_skip_policy.py.  Not done here -- it is a separate change to a
    file this commit does not touch.
  - #177 is still OPEN, so the SCOPE paragraph stands: #178b still lands
    after it.
  - The only textual conflict on the rebase was pytest.ini's testpaths
    list, where #175 had added tools/test_runner_coverage.py.  Resolved
    by keeping all three, in alphabetical order.
  - Neither new module defines a module-level run_tests(), so #175's
    tools/test_runner_coverage.py guard does not claim them; it passes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
(cherry picked from commit 85653f3399db790ac1a6c6e1057ce6cb0bac3505)
@JC-000

JC-000 commented Sep 5, 2026

Copy link
Copy Markdown
Owner Author

Cross-reference: #186 (issue #177, the eighth site in #178's survey) vendors tools/_skip_policy.py byte-identical to this branch's copy — verified against origin/test/178a-skip-policy-rig-half at the time of writing — so it is green standing alone rather than importing a module not yet on master. An identical both-added file merges without conflict in either order; whichever of us lands first, the other's copy of that one file becomes a no-op.

#186 does not duplicate this PR's tools/test_skip_policy.py, tools/test_rig_skip_contract.py, rig wiring, or pytest.ini entries. It adds one pytest.ini line of its own (tools/test_flags_stamp_skip_is_loud.py); if that collides textually with this PR's two, the resolution is to keep all three lines.

If _skip_policy.py changes here before merge, say so and I will drop the file from #186 and rebase.

All four reproduce; each is fixed with a test that is red before and green
after.  Two of them are this PR committing the defect class it exists to
close, which is the reason to state them plainly rather than fold them in.

1. require()'s opt-out CRASHED outside pytest -- and the hand-rolled copy
   it supersedes had already fixed this.

   `import pytest` succeeds whenever pytest is INSTALLED, which on a
   developer machine is always; it says nothing about pytest DRIVING.  So
   `require()` called `pytest.skip()` in the standalone lane too, and
   `pytest.outcomes.Skipped` derives from BaseException, which neither new
   module's `except Exception` runner can catch.  Measured, with the
   sibling harness checkout blocked:

     C64_ALLOW_SKIP=1 python3 tools/test_rig_skip_contract.py
       -> Skipped raised out of require(), traceback, EXIT 1, no summary
          line, and the four source-inspection tests that could have run
          never ran.

   Exit 1 means "a check ran and failed" under this module's own contract,
   so it was wrong in both directions, and only on the escape hatch the PR
   advertises.  Without the opt-out the same run was correct (6 fail, 4
   pass, summary printed) -- which is why nothing caught it.

   tools/test_uci_data_acc.py:631 had already solved exactly this, with
   `sys.modules.get("pytest")` and a comment saying why: "Only hand the
   skip to pytest when pytest is actually driving; the standalone runner
   has its own reporting and must not see Skipped."  A shared helper that
   is WORSE than the copy it supersedes is not a refactor.  Fixed by
   mirroring it: outside pytest, require() now raises VoluntarySkip, a
   plain Exception the running lane can catch.  Either way the test body
   does not run, so the fail-closed property is unchanged.

   The old ImportError branch is gone with it: `sys.modules.get` answers
   "absent" and "not driving" the same way, and both want the same answer.

2. The literal-"1" guard was SKIP-SWALLOWED by the mutation it targets.

   `test_require_still_raises_when_the_opt_out_is_zero` was spelled
   `except SkipPolicyError: pass`, which is vacuous against its own
   defect: revert _opted_out() to bare truthiness and `bool("0")` is true,
   require() takes the opt-out branch, pytest records SKIPPED, exit 0.
   Measured under that mutant: `1 skipped`, not a failure.  The guard
   converted itself into a green skip under precisely the defect it
   guards -- the vacuous-skip shape #178 exists to kill, committed inside
   #178's own implementation, and it read as caught only because three
   sibling tests on the cannot_run() lane happen to fail.

   The catch is now BaseException-wide, because a Skipped here IS the bug.
   Under the same mutant the module now reports 4 failed, 13 passed.

   Two new cases pin defect 1 from both sides: the opt-out must raise
   VoluntarySkip when pytest is not driving, and must still be a pytest
   skip when it is -- so the fix cannot be "simplified" into never
   skipping, which would make the documented hatch a lie.

3. UNDISCLOSED LOOSENING: VICE_HTTPS_OK_TO_RUN unset went 2 -> 0.

   On master, tests/rig_phase3_https_1mhz.py printed "ABORT:
   VICE_HTTPS_OK_TO_RUN is not set." and returned 2, and its docstring
   documented `2 -- pre-flight gate refused`.  This PR replaced both with
   not_applicable() -> 0 and deleted that documented code, while the PR
   body disclosed only the opposite-direction tightening on --u64.

   The first draft's reasoning ("the operator declining an opt-in rig")
   does not survive contact with this repo's own taxonomy.  UNSET IS THE
   DEFAULT STATE: it cannot distinguish a considered decline from a
   forgotten flag, so exit 0 hands a green run to someone who verified
   nothing.  And what the flag asserts -- "the UCI HTTPS listener has
   stopped" -- is a CONTENTION claim, which is the one category this
   policy gives no opt-out at all; the port-443 check three lines below
   is the same interlock measured directly and was already exit 2.

   Reverted to exit 2, non-opt-out-able.  The only thing #178 changes
   about this gate now is that the verdict is a named block instead of a
   bare print.  Pinned behaviourally, including that C64_ALLOW_SKIP=1
   cannot rescue it.

4. tests/rig_phase2_http.py contradicted itself inside forty lines.

   Its module docstring said tests/rig_vice_https_macos.py "owns the
   coverage"; its own _COUNTERPART string -- the one that actually reaches
   the operator -- says that rig drives TLS, "so plaintext HTTP
   specifically has no macOS rig".  The docstring is what a reader hits
   first, and it is load-bearing here: the voluntary-skip verdict is
   justified by "another rig owns the coverage, nothing is lost", which
   for this one rig is false.  Coverage of plaintext HTTP over emulated
   RR-Net really is lost on macOS, at exit 0.

   Exit 0 stays -- there is no remedy on macOS and exit 2 would be a red
   nobody can clear -- but the reason is now the true one, in both the
   docstring and _wrong_platform(), and a new source-inspection test pins
   it so the two halves cannot drift apart again.

5. Minor, and the same root cause as 1: both _standalone() runners did
   `return 1 if failed else 0`, collapsing "could not run" onto 1 and
   contradicting the 0/1/2 contract of the module they test.  They now
   separate VoluntarySkip / SkipPolicyError / Exception and return
   0 / 1 / 2 accordingly.  Measured on tools/test_rig_skip_contract.py
   with the sibling harness blocked: exit 2 plain, exit 0 with
   C64_ALLOW_SKIP=1, exit 0 with the harness present.

Red-then-green, all four:

  D1  restore `import pytest` in require()
        -> test_require_does_not_hand_a_skip_to_a_non_pytest_runner FAILS
  D2  restore bare truthiness in _opted_out()
        -> test_require_still_raises_when_the_opt_out_is_zero FAILS
           (before this commit the same mutant made it SKIP, exit 0)
  D3  re-route the interlock to not_applicable()
        -> test_the_interlock_flag_unset_is_contention_and_stays_exit_two FAILS
  D4  restore the contradictory docstring
        -> test_phase2_does_not_claim_a_counterpart_it_does_not_have FAILS

Bare `pytest` at the repo root: 83 passed (was 80; +3 new cases).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
JC-000 added a commit that referenced this pull request Sep 5, 2026
#180 (#178a) amended the module after this branch copied it, so the two
copies are no longer identical (theirs c359a9b1..., the vendored one
befcf309...) and `git merge-tree` reports an add/add conflict with five
hunks in both directions. The "merges cleanly in either order" claim this
branch shipped with is now false, and the PR body is corrected to match.

Dropping the copy is free: this branch uses only require(), cannot_run()
and EXIT_CANNOT_RUN, whose contracts did not change, and all five probes
in tools/test_flags_stamp_skip_is_loud.py behave identically against
either version of the module.

Merge order is therefore load-bearing and stated in the PR body:
  #183 (this branch's base) -> #180 (brings _skip_policy.py) -> #186.

Also migrates the guard in
test_an_unrelated_option_does_not_suppress_invalidation, added to the
base branch after this one was written, to _require_toolchain(). It was
the one remaining site still spelling the silent-skip prologue by hand,
which is the whole point of #177.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review round 2.  One real hole in a whole-PR claim, plus the fix from
round 1 that shipped without a test of its own.

F1.  "No exit code in this PR is weaker than master's" was FALSE by
     omission, at tools/test_ecdsa_p384_kat.py -- the one wired file the
     round-1 sweep did not touch, so it was never re-checked.

     Master, `--u64` with U64_HOST unset: prints a SKIP line inside the
     u64 branch, but the VICE lane has ALREADY RUN, and main() ends at
     `return 0 if total_fail == 0 else 1`.  A failing emulator vector
     therefore reported 1.

     This branch: the new gate fires before _build_prg() and before any
     VICE work, and carried opt_out_env="C64_ALLOW_SKIP".  With that set
     it returned 0 with nothing run at all.

     The defect is SCOPE, not arithmetic.  C64_ALLOW_SKIP answers "this
     lane has no hardware" -- which is precisely the operator who would
     set it here -- and honouring it at a gate that precedes the VICE
     lane silences the EMULATOR half too, which needs no hardware and
     could have run.  They get exit 0 and never learn the emulator-only
     half regressed.

     Fixed with opt_out_env=None rather than by moving the gate: the
     up-front refusal is the disclosed tightening (~0.1 s instead of
     hours), and moving it below the VICE lane would give that back.  The
     remedy needs no environment variable and costs nothing -- set
     U64_HOST, or drop --u64 and get the VICE lane on its own.  "This
     lane has no hardware" is spelled by NOT PASSING --u64; asking for
     hardware and not supplying it is a malformed invocation, not a
     coverage gap to acknowledge.

     The unreachable backstop later in main() gets the same treatment, so
     one file does not answer the same question two ways.

     Severity was low -- three deliberate operator choices, the block
     prints in full, and _build_prg() would likely fail first because the
     P-384 build has never completed -- but the claim was checkable and
     did not hold, so the code moved rather than the claim.

F3.  D5 (the _standalone() 0/1/2 split) was the one round-1 fix with no
     test.  Nothing collects either runner, so a future edit collapsing
     it back to `return 1 if failed else 0` would go unnoticed -- in a PR
     whose thesis is that a helper with no call-site test is just another
     convention to miss.

     Two subprocess cases in tools/test_skip_policy.py drive the REAL
     runner in tools/test_rig_skip_contract.py with c64_test_harness made
     unimportable by a meta_path finder (the sibling is pip-installed, so
     there is no path entry to remove).  Target chosen deliberately: it
     is the module with a genuine could-not-run lane, and driving it from
     here cannot recurse, because it has no spawner of its own.

       no opt-out       -> exit 2, "CANNOT RUN" and the vacuity string
                           both present, summary line still printed
       C64_ALLOW_SKIP=1 -> exit 0, no traceback, "skipped by explicit
                           opt-out", and the four tests that never needed
                           the sibling checkout still show PASS

     That second case is also the D1 crash scenario, now pinned: before
     the sys.modules fix this exact invocation died with an uncaught
     pytest Skipped at exit 1.

F2.  VoluntarySkip is an Exception, so a consumer's broad
     `except Exception` can swallow it and label it a failure.  Not
     reachable today -- every require() consumer names VoluntarySkip
     before the bare except -- and the direction is the safe one, since a
     swallowed voluntary skip reports as a FAILURE and never as a pass.
     One docstring paragraph says so, and tells a consumer to name it
     first.  Deliberately NOT a BaseException: that is the choice that
     caused D1.

Red-then-green:

  F1  restore opt_out_env="C64_ALLOW_SKIP" on the --u64 gate
        -> test_the_u64_gate_is_not_opt_out_able_because_it_precedes_the_vice_lane
           FAILS (1 failed, 11 passed)
  D5  collapse the runner to `return 1 if failed or cannot else 0`
        -> test_standalone_runner_returns_two_for_could_not_run
           FAILS (1 failed, 18 passed)

Bare `pytest` at the repo root: 86 passed (was 83; +3 new cases).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…te it

76621b6's comment on the new subprocess cases says the target module
"has no spawner of its own", so driving it from here cannot recurse.
True when I reasoned it; false by the time the same commit was written,
because that commit ALSO added a spawner to
tools/test_rig_skip_contract.py -- test_the_u64_gate_* runs
tools/test_ecdsa_p384_kat.py, and _standalone() runs every test_*.  So
the real chain is depth 2 and it fires:

  pytest
    -> test_rig_skip_contract.py   (spawned by test_skip_policy.py)
         -> test_ecdsa_p384_kat.py (spawned by its u64-gate test)

Measured rather than argued: inside the blocked standalone run the
u64-gate case reports PASS, which it can only do by having spawned the
KAT.  The nested run is 0.065 s end to end and exits 2, because the KAT's
gate returns before _build_prg() and before any VICE work.  Timeouts nest
correctly, 180 s outer and 120 s inner.

So: no fork bomb, no hang, nothing to fix in the code.  What needed
fixing is that the comment leaned on an invariant -- "no spawner exists"
-- which is exactly the property that would stop a third link being added
later, and which was no longer true.  A safety argument that has quietly
become false is worse than no argument, because the next person reads it
and stops looking.

The comment now spells the chain out, states the depth, says the bound is
by INSPECTION with no guard in the code, gives the measured cost and the
nesting of the timeouts, and records that the earlier claim went stale
inside one commit -- so a third link has to be added deliberately, in
sight of all that.

Comment-only.  Bare `pytest`: 86 passed, unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
JC-000 added a commit that referenced this pull request Sep 5, 2026
#180 (#178a) amended the module after this branch copied it, so the two
copies are no longer identical (theirs c359a9b1..., the vendored one
befcf309...) and `git merge-tree` reports an add/add conflict with five
hunks in both directions. The "merges cleanly in either order" claim this
branch shipped with is now false, and the PR body is corrected to match.

Dropping the copy is free: this branch uses only require(), cannot_run()
and EXIT_CANNOT_RUN, whose contracts did not change, and all five probes
in tools/test_flags_stamp_skip_is_loud.py behave identically against
either version of the module.

Merge order is therefore load-bearing and stated in the PR body:
  #183 (this branch's base) -> #180 (brings _skip_policy.py) -> #186.

Also migrates the guard in
test_an_unrelated_option_does_not_suppress_invalidation, added to the
base branch after this one was written, to _require_toolchain(). It was
the one remaining site still spelling the silent-skip prologue by hand,
which is the whole point of #177.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
4e90ffc said the spawn chain "terminates because the P-384 KAT spawns
nothing".  Not true in general: _build_prg() runs `make clean` and two
`make BACKEND=uci` invocations at test_ecdsa_p384_kat.py:646-665.  The
next sentence already gave the real reason -- the --u64 gate returns
before _build_prg() -- so the load-bearing fact was present and the depth,
cost and timeout figures were all correct.  What was wrong is the
attribution, and it matters because of what it hides.

The KAT is a leaf ON THIS PATH ONLY, and the path is chosen by where that
gate sits.  So the cost bound of the D5 subprocess test depends on the F1
fix keeping the gate up front.  Move it below _build_prg() -- the
alternative fix considered and rejected when the gate was made
non-opt-out-able -- and the nested run starts a full `make clean && make`
inside a 120 s inner timeout.  Two decisions, one line apart in a file
that mentions neither, and nothing at the gate would tell you.

Both halves now say so:

  - tools/test_skip_policy.py's chain comment names the KAT's `make`
    calls with their line numbers, says the leaf property is a property
    of the PATH and not of the file, and states outright that moving the
    gate is also a decision about this test's cost.
  - tools/test_rig_skip_contract.py -- which ADDS the second link and
    carried no note at all, the comment living only in the module that
    adds the first -- gets the chain drawn at the spawn site, with the
    same warning and a pointer to the fuller note.

A cross-file invariant documented in exactly one of the two files is
half-documented: the reader who breaks it is the one editing the other.

Comment-only.  Bare `pytest`: 86 passed, unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant